Skip to content

fix: correctly return LimitExceeded from reconstruct_storage_map_from_db when start block has too many entries#1825

Merged
mmagician merged 4 commits intorelease/v0.14.0-alphafrom
mmagician-claude/fix-reconstruct-overflow
Mar 23, 2026
Merged

fix: correctly return LimitExceeded from reconstruct_storage_map_from_db when start block has too many entries#1825
mmagician merged 4 commits intorelease/v0.14.0-alphafrom
mmagician-claude/fix-reconstruct-overflow

Conversation

@mmagician
Copy link
Contributor

It turns out that reconstruct_storage_map_from_db was not returning as advertised after the fix in #1816:

    /// Returns:
    ///     - `::LimitExceeded` when too many entries are present

and instead, if block_range_start had too many entries, it would return ::AllEntries


While on it, I also addressed the same pattern as with #1816 where pagination should avoid subtraction on BlockNum.


Note, that a similar pattern also exists for select_transactions_records, but there I don't think there's a way around double allocation of collected values - and it's also a non-issue, because by definition we won't have any transactions at genesis where this could fail. I added a note.

claude added 2 commits March 23, 2026 19:58
…rom_db

The previous pagination fix in `select_account_storage_map_values_paged`
correctly signals "no progress" by returning empty values when a single
block exceeds the entry limit. But the caller
(`reconstruct_storage_map_from_db`) didn't detect this because the
loop exits at the top when `last_block_included == block_num` (both 0
for genesis), before reaching the progress check inside the loop body.

This resulted in returning `AllEntries([])` instead of `LimitExceeded`,
causing the client to import genesis accounts with empty storage maps.

Add an early check after the first page: if no values were returned
and we didn't advance past the starting block, the block has more
entries than the limit allows, so return `LimitExceeded`.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tion

Replace `last_block_num.saturating_sub(1)` with reading block_num from
the last kept `AccountVaultValue`. Same approach as the storage map fix
in #1816.

No unit test added because `select_account_vault_assets` uses a
hardcoded `MAX_ROWS` (~61k) derived from `MAX_RESPONSE_PAYLOAD_BYTES`,
making it impractical to trigger the overflow in a test.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mmagician mmagician changed the title fix: fix: correctly return LimitExceeded from reconstruct_storage_map_from_db when start block has too many entries Mar 23, 2026
Copy link
Collaborator

@igamigo igamigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mmagician mmagician merged commit 24c475c into release/v0.14.0-alpha Mar 23, 2026
15 checks passed
@mmagician mmagician deleted the mmagician-claude/fix-reconstruct-overflow branch March 23, 2026 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants